Skip to content

Conversation

@0xDEnYO
Copy link
Contributor

@0xDEnYO 0xDEnYO commented Dec 19, 2025

Which Jira task belongs to this PR?

https://lifi.atlassian.net/browse/SMAR-99

Why did I implement it this way?

Checklist before requesting a review

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

@lifi-action-bot lifi-action-bot marked this pull request as draft December 19, 2025 05:29
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

Walkthrough

Added incremental TypeChain/ABI scripts and a build orchestration in package.json (new abi:generate:incremental and build:typechain-and-abi); updated abi:generate, typechain:incremental, and confirm-safe-tx script flows. Replaced named bs58 import with default import and call in script/demoScripts/utils/demoScriptHelpers.ts.

Changes

Cohort / File(s) Summary
Package.json — new orchestration & incremental scripts
package.json
Added abi:generate:incremental (bun tasks/generateDiamondABI.ts) and build:typechain-and-abi (bun typechain:incremental && bun abi:generate:incremental).
Package.json — modified scripts
package.json
Updated abi:generate to remove rm -fr typechain/* (now runs forge clean && forge build ... without deleting typechain dir); changed typechain:incremental to prepend forge build src && before duplicate-event removal and typechain generation; changed confirm-safe-tx to run bun build:typechain-and-abi && bunx tsx ./script/deploy/safe/confirm-safe-tx.ts.
Import refactor
script/demoScripts/utils/demoScriptHelpers.ts
Swapped named import decode as decodeBase58 from 'bs58' for default import bs58 and replaced calls decodeBase58(solanaAddress) with bs58.decode(solanaAddress).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review package.json script ordering and new orchestration for unintended redundant builds or missing clean steps.
  • Verify abi:generate:incremental and typechain:incremental commands invoke expected binaries and paths.
  • Check bs58 default import usage and ensure TypeScript typings and runtime behavior remain correct in demoScriptHelpers.ts.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main changes in the PR: fixing an import statement in demoScriptHelpers.ts and updating script dependencies in package.json.
Description check ✅ Passed The description follows the template structure with Jira reference and checklists, but the 'Why did I implement it this way?' section is empty, leaving readers without rationale for the changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-confirm-script

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@0xDEnYO 0xDEnYO marked this pull request as ready for review December 19, 2025 05:30
@0xDEnYO 0xDEnYO enabled auto-merge (squash) December 19, 2025 05:30
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between abe5ed8 and ed8ea40.

📒 Files selected for processing (2)
  • package.json (1 hunks)
  • script/demoScripts/utils/demoScriptHelpers.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
script/demoScripts/**/*.{ts,js}

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

Place TypeScript demo scripts in script/demoScripts/ directory. Reference existing demos for patterns and place helper utilities in script/demoScripts/utils/

Files:

  • script/demoScripts/utils/demoScriptHelpers.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/099-finish.mdc)

After TypeScript changes, run lint/tests with Bun

Files:

  • script/demoScripts/utils/demoScriptHelpers.ts
{src,script,test}/**/*.{sol,ts}

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

Validate all external inputs and configuration (including script/env values) explicitly; prefer existing validation helpers (e.g., Validatable, config readers) over ad-hoc checks

Files:

  • script/demoScripts/utils/demoScriptHelpers.ts
{script,tasks}/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/200-typescript.mdc)

{script,tasks}/**/*.ts: TS scripts use .eslintrc.cjs rules, bunx tsx, citty, consola, and env validated via helpers (e.g., getEnvVar()).
DO NOT use deprecated ethers-based helpers (getProvider, getWalletFromPrivateKeyInDotEnv, ethers sendTransaction, ensureBalanceAndAllowanceToDiamond).
Obey .eslintrc.cjs; avoid any; use TypeChain types from typechain/ directory (e.g., ILiFi.BridgeDataStruct).
Import organization: Group imports as: external libs (viem, consola, citty, dotenv) → TypeChain types → config files → internal utils/helpers. Use type imports for types-only.
Error handling: Use try/catch with consola.error() for logging; exit with process.exit(1) on fatal errors. Provide meaningful error messages.
CLI: use citty; logging via consola; validate env via getEnvVar(); exit 0/1 appropriately.
New TypeScript helpers must be covered by a colocated *.test.ts file using Bun (describe / it / expect) with 100% coverage. Cover edge cases and error paths.

Files:

  • script/demoScripts/utils/demoScriptHelpers.ts
script/demoScripts/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/200-typescript.mdc)

script/demoScripts/**/*.ts: MUST use viem for all contract interactions in demo/operational scripts; ethers.js helpers are deprecated.
Demo scripts follow similar structural patterns (e.g., main() function, setupEnvironment(), helpers from demoScriptHelpers). See demoLidoWrapper.ts, demoUnit.ts, demoEco.ts as reference examples.

Files:

  • script/demoScripts/utils/demoScriptHelpers.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1168
File: script/deploy/_targetState.json:1564-1589
Timestamp: 2025-05-27T12:36:26.987Z
Learning: When reviewing deployment PRs in the lifinance/contracts repository, target state configuration files (like script/deploy/_targetState.json) may be updated for multiple networks even when the PR is focused on deploying to a specific network. The scope should be determined by the PR title and description, not just by all configuration changes present in the files.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: script/deploy/facets/LDA/DeployAlgebraFacet.s.sol:4-4
Timestamp: 2025-08-26T15:19:07.800Z
Learning: DeployScriptBase.sol is located at script/deploy/facets/utils/DeployScriptBase.sol, not script/deploy/utils/DeployScriptBase.sol. Import paths from script/deploy/facets/LDA/ should use "../utils/DeployScriptBase.sol" to reference it correctly.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 953
File: script/deploy/zksync/utils/UpdateScriptBase.sol:32-50
Timestamp: 2025-03-05T14:41:05.791Z
Learning: In the LiFi contracts repository, explicit error handling for file operations and address validation in deployment scripts like UpdateScriptBase.sol is not considered necessary by the maintainer.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1169
File: script/deploy/zksync/DeployFeeCollector.zksync.s.sol:32-37
Timestamp: 2025-05-27T12:00:43.940Z
Learning: The lifinance/contracts repository has deployment scripts that perform validation checks (including zero-address validation) before executing individual deploy scripts, making runtime validation checks in the deploy scripts themselves redundant.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1406
File: script/deploy/facets/UpdateUnitFacet.s.sol:1-3
Timestamp: 2025-10-02T18:14:45.047Z
Learning: For update scripts in script/deploy/facets/, the SPDX license identifier should be immediately followed by the pragma statement without a blank line in between, as confirmed by mirooon for script/deploy/facets/UpdateUnitFacet.s.sol.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:334-334
Timestamp: 2025-07-17T04:21:55.549Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep private key processing simple in scripts like execute-pending-timelock-tx.ts without adding format validation, prioritizing code simplicity over strict validation in controlled environments.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:129-129
Timestamp: 2025-07-17T04:21:50.790Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers not to add private key format validation in deployment scripts like execute-pending-timelock-tx.ts, prioritizing code simplicity over strict validation in their controlled environment.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/001-project-structure.mdc:0-0
Timestamp: 2025-12-17T10:27:05.468Z
Learning: Applies to script/demoScripts/**/*.{ts,js} : Place TypeScript demo scripts in `script/demoScripts/` directory. Reference existing demos for patterns and place helper utilities in `script/demoScripts/utils/`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to script/demoScripts/**/*.ts : MUST use viem for all contract interactions in demo/operational scripts; ethers.js helpers are deprecated.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1299
File: script/demoScripts/demoAcrossV4.ts:728-737
Timestamp: 2025-08-22T10:03:58.794Z
Learning: Demo scripts in `script/demoScripts` are exempt from the citty CLI argument parsing requirement that applies to other TypeScript scripts. Templates for demo scripts don't use citty and this is acceptable.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to script/demoScripts/**/*.ts : Demo scripts follow similar structural patterns (e.g., `main()` function, `setupEnvironment()`, helpers from `demoScriptHelpers`). See `demoLidoWrapper.ts`, `demoUnit.ts`, `demoEco.ts` as reference examples.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Always reuse existing helpers and types: Search `script/common/`, `script/utils/`, `script/demoScripts/utils/`, and other helper directories before implementing new functionality. Key helpers: `script/utils/deploymentHelpers.ts` (deployment loading), `script/demoScripts/utils/demoScriptHelpers.ts` (viem-based demo helpers and swap helpers), `script/common/types.ts` (shared types).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : DO NOT use deprecated ethers-based helpers (`getProvider`, `getWalletFromPrivateKeyInDotEnv`, ethers `sendTransaction`, `ensureBalanceAndAllowanceToDiamond`).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : CLI: use `citty`; logging via `consola`; validate env via `getEnvVar()`; exit 0/1 appropriately.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : Obey `.eslintrc.cjs`; avoid `any`; use TypeChain types from `typechain/` directory (e.g., `ILiFi.BridgeDataStruct`).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : Import organization: Group imports as: external libs (viem, consola, citty, dotenv) → TypeChain types → config files → internal utils/helpers. Use `type` imports for types-only.
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: script/demoScripts/demoGlacisAirlift.ts:42-59
Timestamp: 2025-01-22T12:40:50.355Z
Learning: In TypeScript/JavaScript demo scripts using ethers.js, nonce is automatically managed by the library and doesn't need to be explicitly set. Similarly, the default gas limit is typically sufficient for standard operations like ERC20 approvals.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to script/demoScripts/**/*.ts : MUST use viem for all contract interactions in demo/operational scripts; ethers.js helpers are deprecated.

Applied to files:

  • script/demoScripts/utils/demoScriptHelpers.ts
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : DO NOT use deprecated ethers-based helpers (`getProvider`, `getWalletFromPrivateKeyInDotEnv`, ethers `sendTransaction`, `ensureBalanceAndAllowanceToDiamond`).

Applied to files:

  • package.json
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : Obey `.eslintrc.cjs`; avoid `any`; use TypeChain types from `typechain/` directory (e.g., `ILiFi.BridgeDataStruct`).

Applied to files:

  • package.json
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : CLI: use `citty`; logging via `consola`; validate env via `getEnvVar()`; exit 0/1 appropriately.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : TS scripts use `.eslintrc.cjs` rules, `bunx tsx`, `citty`, `consola`, and env validated via helpers (e.g., `getEnvVar()`).

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:28:51.714Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/107-solidity-scripts.mdc:0-0
Timestamp: 2025-12-17T10:28:51.714Z
Learning: Applies to script/deploy/**/*.sol : Keep deployment scripts under `script/deploy/` directory structure (and `script/deploy/zksync/` when relevant) and reuse helper/config files like `deployRequirements.json` and `targetState.json` instead of creating new patterns

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : New TypeScript helpers must be covered by a colocated `*.test.ts` file using Bun (`describe` / `it` / `expect`) with 100% coverage. Cover edge cases and error paths.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:27:39.700Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/099-finish.mdc:0-0
Timestamp: 2025-12-17T10:27:39.700Z
Learning: Applies to **/*.sol : After Solidity changes, run `forge test` (or note suites remaining)

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:05.622Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:05.622Z
Learning: Applies to {script,tasks}/**/*.ts : Import organization: Group imports as: external libs (viem, consola, citty, dotenv) → TypeChain types → config files → internal utils/helpers. Use `type` imports for types-only.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:27:39.700Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/099-finish.mdc:0-0
Timestamp: 2025-12-17T10:27:39.700Z
Learning: Applies to **/*.{ts,tsx} : After TypeScript changes, run lint/tests with Bun

Applied to files:

  • package.json
📚 Learning: 2024-12-02T08:19:07.783Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 875
File: script/utils/fetch-missing-deployments.ts:46-46
Timestamp: 2024-12-02T08:19:07.783Z
Learning: In our codebase, scripts like `script/utils/fetch-missing-deployments.ts` are intended to run in Node.js version 18 or newer, so global `fetch` is available without importing.

Applied to files:

  • package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: run-unit-tests
  • GitHub Check: run-unit-tests
🔇 Additional comments (2)
script/demoScripts/utils/demoScriptHelpers.ts (2)

150-164: LGTM - Usage updated correctly.

The function call has been updated to use bs58.decode() method, which is correct for the default import pattern. The logic remains sound, assuming the import verification passes.


5-7: No issue detected — default import is the standard pattern for bs58.

The code correctly uses import bs58 from 'bs58', which aligns with official bs58 documentation and is properly supported by modern versions used via the transitive dependency. The @ts-expect-error and eslint-disable comments appropriately suppress warnings for this transitive dependency access pattern.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
package.json (1)

107-107: Clarify when incremental builds require full regeneration.

The typechain:incremental script skips forge clean and uses a filtered glob pattern (out/!(*.t).sol/*.json), which is appropriate for speed. TypeChain always rewrites existing files, so contract updates are handled correctly. However, if contracts are renamed or deleted, their old TypeChain-generated files will remain until a full bun typechain build is run.

Add a comment or documentation note explaining:

  • Use bun typechain:incremental for local development (faster)
  • Use bun typechain (full rebuild) when renaming/deleting contracts or before committing
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ed8ea40 and 658bc30.

📒 Files selected for processing (1)
  • package.json
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1168
File: script/deploy/_targetState.json:1564-1589
Timestamp: 2025-05-27T12:36:26.987Z
Learning: When reviewing deployment PRs in the lifinance/contracts repository, target state configuration files (like script/deploy/_targetState.json) may be updated for multiple networks even when the PR is focused on deploying to a specific network. The scope should be determined by the PR title and description, not just by all configuration changes present in the files.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 953
File: script/deploy/zksync/utils/UpdateScriptBase.sol:32-50
Timestamp: 2025-03-05T14:41:05.791Z
Learning: In the LiFi contracts repository, explicit error handling for file operations and address validation in deployment scripts like UpdateScriptBase.sol is not considered necessary by the maintainer.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: script/deploy/facets/LDA/DeployAlgebraFacet.s.sol:4-4
Timestamp: 2025-08-26T15:19:07.800Z
Learning: DeployScriptBase.sol is located at script/deploy/facets/utils/DeployScriptBase.sol, not script/deploy/utils/DeployScriptBase.sol. Import paths from script/deploy/facets/LDA/ should use "../utils/DeployScriptBase.sol" to reference it correctly.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1406
File: script/deploy/facets/UpdateUnitFacet.s.sol:1-3
Timestamp: 2025-10-02T18:14:45.047Z
Learning: For update scripts in script/deploy/facets/, the SPDX license identifier should be immediately followed by the pragma statement without a blank line in between, as confirmed by mirooon for script/deploy/facets/UpdateUnitFacet.s.sol.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1169
File: script/deploy/zksync/DeployFeeCollector.zksync.s.sol:32-37
Timestamp: 2025-05-27T12:00:43.940Z
Learning: The lifinance/contracts repository has deployment scripts that perform validation checks (including zero-address validation) before executing individual deploy scripts, making runtime validation checks in the deploy scripts themselves redundant.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:129-129
Timestamp: 2025-07-17T04:21:50.790Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers not to add private key format validation in deployment scripts like execute-pending-timelock-tx.ts, prioritizing code simplicity over strict validation in their controlled environment.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:334-334
Timestamp: 2025-07-17T04:21:55.549Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep private key processing simple in scripts like execute-pending-timelock-tx.ts without adding format validation, prioritizing code simplicity over strict validation in controlled environments.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : Obey `.eslintrc.cjs`; avoid `any`; use TypeChain types from `typechain/` directory (e.g., `ILiFi.BridgeDataStruct`).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : Import organization: Group imports as: external libs (viem, consola, citty, dotenv) → TypeChain types → config files → internal utils/helpers. Use `type` imports for types-only.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/001-project-structure.mdc:0-0
Timestamp: 2025-12-17T10:27:05.481Z
Learning: Applies to script/demoScripts/**/*.{ts,js} : Place TypeScript demo scripts in `script/demoScripts/` directory. Reference existing demos for patterns and place helper utilities in `script/demoScripts/utils/`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : CLI: use `citty`; logging via `consola`; validate env via `getEnvVar()`; exit 0/1 appropriately.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: Always reuse existing helpers and types: Search `script/common/`, `script/utils/`, `script/demoScripts/utils/`, and other helper directories before implementing new functionality. Key helpers: `script/utils/deploymentHelpers.ts` (deployment loading), `script/demoScripts/utils/demoScriptHelpers.ts` (viem-based demo helpers and swap helpers), `script/common/types.ts` (shared types).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: Applies to script/demoScripts/**/*.ts : MUST use viem for all contract interactions in demo/operational scripts; ethers.js helpers are deprecated.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: Always check for existing types before defining new ones: Search in order: TypeChain types (`typechain/`), viem types, `script/common/types.ts`, domain-specific type files (e.g., `script/troncast/types.ts`, `script/deploy/tron/types.ts`).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: Applies to {script,tasks}/**/*.ts : DO NOT use deprecated ethers-based helpers (`getProvider`, `getWalletFromPrivateKeyInDotEnv`, ethers `sendTransaction`, `ensureBalanceAndAllowanceToDiamond`).
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to script/demoScripts/**/*.ts : Demo scripts follow similar structural patterns (e.g., `main()` function, `setupEnvironment()`, helpers from `demoScriptHelpers`). See `demoLidoWrapper.ts`, `demoUnit.ts`, `demoEco.ts` as reference examples.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/107-solidity-scripts.mdc:0-0
Timestamp: 2025-12-17T10:28:51.722Z
Learning: Applies to script/deploy/**/*.sol : Keep deployment scripts under `script/deploy/` directory structure (and `script/deploy/zksync/` when relevant) and reuse helper/config files like `deployRequirements.json` and `targetState.json` instead of creating new patterns
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2025-12-17T10:29:06.121Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: Applies to {script,tasks}/**/*.ts : DO NOT use deprecated ethers-based helpers (`getProvider`, `getWalletFromPrivateKeyInDotEnv`, ethers `sendTransaction`, `ensureBalanceAndAllowanceToDiamond`).

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:06.122Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : Obey `.eslintrc.cjs`; avoid `any`; use TypeChain types from `typechain/` directory (e.g., `ILiFi.BridgeDataStruct`).

Applied to files:

  • package.json
📚 Learning: 2024-11-21T08:23:50.099Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 861
File: script/deploy/safe/add-owners-to-safe.ts:48-48
Timestamp: 2024-11-21T08:23:50.099Z
Learning: In the script `script/deploy/safe/add-owners-to-safe.ts`, additional defensive checks for network configuration may be unnecessary because the script will fail anyway when the network configuration is missing.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:28:40.640Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/105-security.mdc:0-0
Timestamp: 2025-12-17T10:28:40.640Z
Learning: Applies to {src,script}/**/*.sol : Avoid introducing new external call patterns in facets or scripts without checking existing libraries (`LibAsset`, `LibSwap`, `LibAllowList`) for prior art

Applied to files:

  • package.json
📚 Learning: 2024-11-04T03:50:06.443Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 847
File: script/tasks/fundNewWalletOnAllChains.ts:179-187
Timestamp: 2024-11-04T03:50:06.443Z
Learning: In the `script/tasks/fundNewWalletOnAllChains.ts` file, adding a timeout to the transaction confirmation wait using `publicClient.waitForTransactionReceipt` is not required.

Applied to files:

  • package.json
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/polygon.json:0-0
Timestamp: 2024-10-09T03:47:21.269Z
Learning: Ensure that scripts used for codebase verification produce valid and accurate results before reporting issues, especially when checking Ethereum address checksums in `deployments/polygon.json`.

Applied to files:

  • package.json
📚 Learning: 2025-01-28T14:29:00.823Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 924
File: script/deploy/zksync/utils/UpdateScriptBase.sol:112-178
Timestamp: 2025-01-28T14:29:00.823Z
Learning: The suggestion to modify `buildDiamondCut` function in `UpdateScriptBase.sol` to handle selectors from multiple old facets differently was deemed unnecessary by the maintainer.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:28:40.640Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/105-security.mdc:0-0
Timestamp: 2025-12-17T10:28:40.640Z
Learning: Applies to {src,script}/**/*.sol : Do not weaken existing access controls, timelock flows, or Safe multisig protections; any change that touches admin-only functionality must call out its governance impact

Applied to files:

  • package.json
📚 Learning: 2025-06-13T08:30:28.665Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1207
File: script/deploy/facets/DeployPioneerFacet.s.sol:17-27
Timestamp: 2025-06-13T08:30:28.665Z
Learning: Deployment scripts rely on external automation (pre-script checks) to validate constructor arguments such as non-zero addresses, so adding explicit `require` guards inside the Foundry deployment scripts is generally unnecessary.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:06.121Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: Applies to {script,tasks}/**/*.ts : TS scripts use `.eslintrc.cjs` rules, `bunx tsx`, `citty`, `consola`, and env validated via helpers (e.g., `getEnvVar()`).

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:06.122Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : CLI: use `citty`; logging via `consola`; validate env via `getEnvVar()`; exit 0/1 appropriately.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:27:39.709Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/099-finish.mdc:0-0
Timestamp: 2025-12-17T10:27:39.709Z
Learning: Applies to **/*.sol : After Solidity changes, run `forge test` (or note suites remaining)

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:06.122Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : Import organization: Group imports as: external libs (viem, consola, citty, dotenv) → TypeChain types → config files → internal utils/helpers. Use `type` imports for types-only.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:06.122Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.122Z
Learning: Applies to {script,tasks}/**/*.ts : New TypeScript helpers must be covered by a colocated `*.test.ts` file using Bun (`describe` / `it` / `expect`) with 100% coverage. Cover edge cases and error paths.

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:27:39.709Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/099-finish.mdc:0-0
Timestamp: 2025-12-17T10:27:39.709Z
Learning: Applies to **/*.{ts,tsx} : After TypeScript changes, run lint/tests with Bun

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:28:51.722Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/107-solidity-scripts.mdc:0-0
Timestamp: 2025-12-17T10:28:51.722Z
Learning: Applies to script/deploy/**/*.sol : Keep deployment scripts under `script/deploy/` directory structure (and `script/deploy/zksync/` when relevant) and reuse helper/config files like `deployRequirements.json` and `targetState.json` instead of creating new patterns

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:40.543Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/402-typescript-tests.mdc:0-0
Timestamp: 2025-12-17T10:29:40.543Z
Learning: After TypeScript test changes, run Bun tests (or state which suites remain)

Applied to files:

  • package.json
📚 Learning: 2025-12-17T10:29:06.121Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/200-typescript.mdc:0-0
Timestamp: 2025-12-17T10:29:06.121Z
Learning: After TS changes run lint/tests to ensure eslint/type checks pass.

Applied to files:

  • package.json
📚 Learning: 2025-06-05T10:00:01.583Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1145
File: script/tasks/diamondSyncWhitelistedAddresses.sh:208-209
Timestamp: 2025-06-05T10:00:01.583Z
Learning: Task scripts in script/tasks/ directory (like diamondSyncWhitelistedAddresses.sh) define functions that are sourced by deployAllContracts.sh and called from there, rather than executing directly. They don't need to call their own functions at the end of the file.

Applied to files:

  • package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: run-unit-tests
🔇 Additional comments (2)
package.json (2)

60-62: Good addition of incremental build scripts!

The new incremental scripts (abi:generate:incremental and build:typechain-and-abi) provide a faster alternative while preserving the full clean build option in abi:generate. The orchestration in build:typechain-and-abi properly chains the incremental typechain and ABI generation steps.


68-68: Excellent fix for the double-cleaning issue!

The confirm-safe-tx script now uses the new build:typechain-and-abi orchestration, which eliminates the double forge clean execution that was flagged in the previous review. The incremental workflow ensures artifacts are built once and reused efficiently.

@0xDEnYO 0xDEnYO merged commit e901cf2 into main Dec 22, 2025
32 of 36 checks passed
@0xDEnYO 0xDEnYO deleted the fix-confirm-script branch December 22, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants